hello hi just tagging in so I know when to merge #9153

@asheemmamoowala Will the de-duplication of wrapped tiles in TilePyramid here play nice with your ImageSource changes?

@jfirebaugh Could you take a look at how I did selective binding fill_color/halo_color? I don't have it compiling yet on gcc, but I'm very uncertain about the approach I'm using. I think ideally maybe you could have the same vertex buffers but two sets of paint property binders you could pass in to the draw call in painter_symbol.cpp. I had a lot of trouble trying to tease that logic apart without dramatically changing how the binders work, and that seemed like overkill -- unless "selective binding" is a generalizable idea we'd want to use elsewhere? Still, my current implementation has some code smells: (1) setting a global flag in Context to enable the "selective binding", and (2) making the ordering of IconPaintProperties/TextPaintProperties magical (you need to put the "alternate" property at the end of the list in order to generate the right attribute locations.

@ChrisLoer - #8968 doesn't use OverscaledTileID, TilePyramid, or update_renderables so I think your changes to wrapped tiles shouldn't have any impact on ImageSource.

Thanks @asheemmamoowala!

@ivovandongen This should be ready to test against the mythical 8-attribute device. Just a sanity check that labels have fill and halo colors should be enough, although I've also been testing the second pathway where fill/halo color are defined as a zoom function (had to make a custom style to test that).

I'd hope to be able to do selective binding without modifying the low level OpenGL wrappers in mbgl/gl/attribute.cpp. The changes here are adding a pretty specialized case to what should be generic code.

I think the core requirement is to enable programs that bind a subset of attributes in a vertex array. Here's my attempt. The main blocker is the fact that Segment is templated on an Attributes type that must be the same as the Attributes type used by any Program that wants to draw those segments. The reason this is done is so that Segment can track the attribute bindings, and if they are different than the previous draw call, update the VAO state. The reason that this must be tracked on a per-Segment basis is that the VAO state effectively includes the offset from the beginning of the vertex buffer for that Segment.

Options:

• The ARB_vertex_attrib_binding extension allows factoring this offset out. I believe this would allow Program to track the binding state instead of Segment. This would be ideal, but I doubt the extension is available everywhere.
• Move the tracking of attribute bindings to Program anyway, and rebind when the vertex offset changes. This would nullify the performance advantage of VAOs in the case that a vertex buffer has more than 2^16 vertices -- rare, but it happens.
• Allow Segment to track binding state for more than one Program type. Relax the requirement that its Attributes type match the Programs Attributes -- the Programs would merely need to be a subset. I think a previous iteration of VAO binding actually did this -- the moral equivalent of today's Segment was templated on the Program types that were going to use it, and there was a separate VAO for each one.

The second option sounds best to me right now.

@ChrisLoer The text/halo colors seem to work. But there is some general regressions in rendering on this device that also shows up in label rendering it seems:

@jfirebaugh OK, I took a stab at removing the Segment specialization on Attributes. I haven't figured out how to re-enable the VAO optimization yet. Is this in the direction you were thinking?

I think the core requirement is to enable programs that bind a subset of attributes in a vertex array.

If we implement text and halo rendering in a single draw call is this still necessary? mapbox/mapbox-gl-js#4709 (comment)

Are we going to hit the attribute limit for data-driven patterned/dashed lines? Will this require a different solution, like somehow packing multiple style properties in a single attribute?

If we implement text and halo rendering in a single draw call is this still necessary?

If we do them in a single draw call, we have to pass both values down, so the selective binding wouldn't be possible any more. We'd have to get around the attribute limit some other way. Looking at your changes in mapbox/mapbox-gl-js#4781, it looks like there are 9 attributes but as much as 72 bits open?

• Kind of 16 bits could be put in a_projected_pos[3] although the float/int thing is weird as is the dynamic updating.
• a_data has 24 bits free.
• a_size has 32 bits free, I think?

All the pitch-scaling related changes look good to me

a_size has 32 bits free, I think?

Yep, but since we should move towards treating a_size as a regular data-driven property we might not want to pack into it.